Added property version migration #4330
Conversation
…upIdAndArtifactId and ChangeManagedDependencyGroupIdAndArtifactId
…DependencyGroupIdAndArtifactId
…into feature/ChangePropertyVersions
|
Hi @marcel-gepardec ; Appreciate the effort you've put in here. I do wonder if this would be better as a separate recipe though, as to not complicate the options and logic further for these already not trivial recipes. There's a lot to factor in, such as parents, BOMs and their order, managed dependencies and properties. To add optional renaming on top of that adds complexity which isn't often used, especially considering that these recipes are a often include in framework migrations where the option is not set. That way folks would still see properties not adhering to this specification. Comparatively I'd imagine a separate recipe that only renames properties from whatever they are named currently to a prefix/suffix naming scheme might be easier to maintain going forward. And even then we'd need to take into account if a property is actually overriding a property as the often used Spring managed dependency version properties, and handle that appropriately. What are your thoughts on the above? |
|
My thoughts on this: There are two issues here:
I also think those two should be separated in different recipes. The first seems to me as expected behavior. It may lead to problems if two or more dependencies use the same property, but after migration the version numbers differ for some reason. What should the recipe do? Override the first change, how high is the risk of breaking things? Change the version tag instead, how complicated is this to implement? |
|
Thanks for clarifying! Indeed best to separate those two concerns. I'd have expected 1. to already be handled with the recipe we have, also based on the tests & changes I've seen up to now. If this isn't the case then a separate unit test to replicate the issue is probably best, and we can then work from there. Let's split off the 2. concern into a separate issue, if we think it's feasible to implement at all in a reliable manner without supervision. Best not to make those changes part of this PR, however well intended. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
Closing as discussed on #4366 (comment) |
Property version migration
What's changed?
Depending on the content inside the version tag the migration decides if the version is changed inside the dependency tag or inside the properties. I also added a new option to the ChangeDependencyGroupIdAndArtifactId recipe implementation.
The new option is changePropertyVersionNames. It is set per default on false. If it is set on true than it changes the property variable name from whatsoever to the new [artifactId].version
Example
Recipe with changePropertyVersionNames on true:
Before:
... <properties> <javax.persistence.version>2.2</javax.persistence.version> </properties> <dependencyManagement> <dependencies> <dependency> <groupId>javax.persistence</groupId> <artifactId>javax.persistence-api</artifactId> <version>${javax.persistence.version}</version> </dependency> </dependencies> </dependencyManagement> <dependencies> <dependency> <groupId>javax.persistence</groupId> <artifactId>javax.persistence-api</artifactId> </dependency> </dependencies> ...After:
... <properties> <jakarta.persistence-api.version>3.2.0</jakarta.persistence-api.version> </properties> <dependencyManagement> <dependencies> <dependency> <groupId>jakarta.persistence</groupId> <artifactId>jakarta.persistence-api</artifactId> <version>${jakarta.persistence-api.version}</version> </dependency> </dependencies> </dependencyManagement> <dependencies> <dependency> <groupId>jakarta.persistence</groupId> <artifactId>jakarta.persistence-api</artifactId> </dependency> </dependencies> ...What's your motivation?
While migrating one of my projects I noticed that in my pom every single version inside the dependencies changed from a property variable to a normal version.